Skip to content

gh-149534: Fix unification of defaultdict and frozendict with |#149539

Open
sobolevn wants to merge 1 commit intopython:mainfrom
sobolevn:issue-149534
Open

gh-149534: Fix unification of defaultdict and frozendict with |#149539
sobolevn wants to merge 1 commit intopython:mainfrom
sobolevn:issue-149534

Conversation

@sobolevn
Copy link
Copy Markdown
Member

@sobolevn sobolevn commented May 8, 2026

I have a lot of free time today 😆

@sobolevn sobolevn requested a review from JelleZijlstra May 8, 2026 06:12
@sobolevn sobolevn requested a review from rhettinger as a code owner May 8, 2026 06:12
@sobolevn sobolevn added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label May 8, 2026
with self.assertRaises(TypeError):
i |= None

# frozendict
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also test what frozendict | defaultdict does?

@sobolevn
Copy link
Copy Markdown
Member Author

sobolevn commented May 9, 2026

@JelleZijlstra you were right, there's a big behavior difference here.

>>> from collections import OrderedDict
>>> dict() | OrderedDict()
OrderedDict()
>>> frozendict() | OrderedDict()
frozendict()

Since OrderedDict and defaultdict are direct subtypes of dict, _PyDict_Or is never called for the first case.

cpython/Objects/typeobject.c

Lines 10531 to 10539 in 354ef33

#define SLOT1BINFULL(FUNCNAME, TESTFUNC, SLOTNAME, DUNDER, RDUNDER) \
static PyObject * \
FUNCNAME(PyObject *self, PyObject *other) \
{ \
PyObject* stack[2]; \
PyThreadState *tstate = _PyThreadState_GET(); \
int do_other = !Py_IS_TYPE(self, Py_TYPE(other)) && \
Py_TYPE(other)->tp_as_number != NULL && \
Py_TYPE(other)->tp_as_number->SLOTNAME == TESTFUNC; \

While it is called for frozendict.

I am not sure that this is a bug, though. It works as expected,, if you know all the little details. But, it might be unexpected for people who just want to swap dict to frozendict. The end result might be different.

@vstinner, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.15 pre-release feature fixes, bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants